Clamp to 3D Tiles most detailed #7115

lilleyse · 2018-10-03T23:19:10Z

This builds on the first 3D Tiles clamping PR and adds new functions for getting the most detailed height of 3D Tiles, on screen or off screen. It does this by loading the needed leaf tiles asynchronously and then calling the synchronous clampToHeight, sampleHeight functions. It returns a promise to the result.

The new functions are:

scene.pickFromRayMostDetailed (private)
scene.drillPickFromRayMostDetailed (private)
scene.sampleHeightMostDetailed
scene.clampToHeightMostDetailed

I still have some more cleanup work to do, but this is ready to experiment with. I'll update once I think it's in a good enough state to review.

@pjcozzi I'm really starting to see what you mean about moving these pick functions out of Scene. It only got worse.

To do:

Cleanup
CHANGES.md
Cesium3DTileset unit tests
Decide what to do for descendant traversal - Clamp to 3D Tiles most detailed #7115 (comment)
Move pick code out of Scene. Might be a follow up PR.

Fixes #7120

cesium-concierge · 2018-10-03T23:19:12Z

Thanks for the pull request @lilleyse!

✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

Cesium Viewer works.
Works in 2D/CV.
Works (or fails gracefully) in IE11.

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

…cted in updates.

pjcozzi · 2018-10-07T00:41:33Z

@OmarShehata could you give this a shot to test and review once @lilleyse says its ready?

In the meantime, I'll have a look at the Sandcastle example.

pjcozzi · 2018-10-07T00:45:57Z

Apps/Sandcastle/gallery/Clamp to 3D Tiles Offscreen.html

@@ -0,0 +1,104 @@
+<!DOCTYPE html>


Are we sure we should call this "off screen?" What speaks to the user best? Async? Most Detailed?

Really just that comment for now, this is cool, and could be used for profiles, e.g., change 30 to 300.

I'll also look at the implementation when ready.

While I do support having smaller, more focused Sandcastle examples in general, in this case I wonder if it would be better to combine this with the existing Clamp to 3D Tiles example, in the same way the terrain sample and sampleMostDetailed are in the same example:

http://localhost:8080/Apps/Sandcastle/index.html?src=Terrain.html

"Most detailed" is probably the most relevant to the user, but I can imagine users running into this issue when they realize sampleHeight only works for already visible tiles and want to figure out how to do it for tiles offscreen. I think in any case the goal for the user is just clamping to 3D Tiles/getting tiles height, so a combined example geared towards that use case makes the most sense to me.

I think keeping them separate is better because they represent different enough use cases.

But maybe the new demo should be called Sample Height from 3D Tiles. The main difference being this demo samples some heights and shows the result while the first demo actually physically clamps something to the tileset.

I think that makes more sense! I think that naming will make it easier to find.

OmarShehata · 2018-10-09T20:27:58Z

I just tested this out, and it seems to work pretty well! I'd say the only thing I was confused about with using it, when I copy pasted the example code to use in my own code, it took me a while to realize that it was returning a single Cartesian when I pass a single Cartesian. It seems less error-prone to me to just always take an array and return an array.

I also noticed that when I clamp a position to a tile, it sometimes forces that tile to load before the parent(s). Is that intentional? So for example, on the BIM tileset (localhost:8080/...) I can make the big tower load first by clamping a position to it.

sherousee · 2018-10-15T15:10:49Z

I tested scene.pickFromRayMostDetailed(), and it is fantastic. Can't wait to start using. When do you think we will see this merged and released?

…mpleHeight and clampToHeight

lilleyse · 2018-10-15T20:43:19Z

I just tested this out, and it seems to work pretty well! I'd say the only thing I was confused about with using it, when I copy pasted the example code to use in my own code, it took me a while to realize that it was returning a single Cartesian when I pass a single Cartesian. It seems less error-prone to me to just always take an array and return an array.

Yeah better to be consistent here. I changed it to use arrays only.

I'm also trying to decide whether sampleHeightMostDetailed and clampToHeightMostDetailed should modify the positions in place like sampleTerrain(MostDetailed) would. I am leaning towards "yes".

So far the analogs are:

Sync functions:
Globe.getHeight (Takes single cartographic, returns height, does not modify cartographic in place)
Scene.sampleHeight (Same as above)
Scene.clampToHeight (Takes single cartesian, optional result parameter stores result (user can set it to the input cartesian if they want to modify it in place))

Async functions:
sampleTerrain (Takes array, modifies cartographic positions in place, sets .height to undefined if there was nothing to sample)
sampleTerrainMostDetailed (Same as above)
Scene.sampleHeightMostDetailed (Same as above)
Scene.clampToHeightMostDetailed (Same as above but with cartesians. Sets the array element to undefined if there was nothing to sample)

lilleyse · 2018-10-15T21:39:57Z

I also noticed that when I clamp a position to a tile, it sometimes forces that tile to load before the parent(s). Is that intentional? So for example, on the BIM tileset (localhost:8080/...) I can make the big tower load first by clamping a position to it.

Ah.... that's a weird edge case.

It's happening because if a tile is selected but neither it nor any of its ancestors are loaded it tries to selected any loaded descendants to help fill in the gaps. And it does this even if the tile wouldn't normally refine to those descendants. This is mainly designed for when immediatelyLoadDesiredLevelOfDetail is true and you quickly zoom out.

I have to decide whether this is worth addressing or just leaving. Possibly the descendant refinement should be reserved for when immediatelyLoadDesiredLevelOfDetail is true.

lilleyse · 2018-10-15T21:41:38Z

I tested scene.pickFromRayMostDetailed(), and it is fantastic. Can't wait to start using. When do you think we will see this merged and released?

Great to hear! I'm trying to finish it up now so it will have enough time to sit in master before a 1.51 release.

lilleyse · 2018-11-06T01:01:57Z

Added tileset tests and incorporated your suggestions @pjcozzi. This is ready for another review.

pjcozzi · 2018-11-06T01:43:59Z

Is the tasklist up-to-date?

CHANGES.md
Cesium3DTileset unit tests
Decide what to do for descendant traversal - #7115 (comment)
Move pick code out of Scene. Might be a follow up PR.

pjcozzi · 2018-11-06T02:12:43Z

@lilleyse just curious - did you try this with a point cloud?

pjcozzi · 2018-11-06T02:13:04Z

All looks good to me, even tested in Safari.

@lilleyse any last considerations or OK to merge?

lilleyse · 2018-11-07T20:57:07Z

@lilleyse just curious - did you try this with a point cloud?

I tried now and unfortunately got what I was afraid of happening - it picked through the point cloud despite requesting the high detail tiles. localhost:8080 Example.

We had ideas for fixing this, like rendering to a larger than 1x1 canvas with attenuation turned on. Is this something we should write an issue for and do later?

lilleyse · 2018-11-07T21:17:38Z

Decide what to do for descendant traversal - #7115 (comment)

I think I'm going to leave this one because it's more of an oddity than a legitimate bug, and only shows up in the BIM tileset because the root tile is so large and slow to download.

lilleyse · 2018-11-07T21:18:43Z

Opened #7247 and #7246 for cleanup related comments.

lilleyse · 2018-11-07T21:21:25Z

@pjcozzi The task list is updated and the code is ready. Point cloud support would be the only thing holding this up.

pjcozzi · 2018-11-07T22:07:28Z

@lilleyse just curious - did you try this with a point cloud?

I tried now and unfortunately got what I was afraid of happening - it picked through the point cloud despite requesting the high detail tiles. localhost:8080 Example.

We had ideas for fixing this, like rendering to a larger than 1x1 canvas with attenuation turned on. Is this something we should write an issue for and do later?

Please submit an issue.

pjcozzi · 2018-11-07T22:08:24Z

Is appveyor expected to fail right now?

pjcozzi · 2018-11-07T22:12:35Z

Is appveyor expected to fail right now?

If so, this is ready to merge.

lilleyse · 2018-11-07T23:58:55Z

Merged in master and CI passed.

pjcozzi · 2018-11-08T00:27:24Z

🚀

lilleyse added 6 commits September 18, 2018 16:12

Move common code into Cesium3DTile

eb4b83d

Pass statistics around in preparation for offscreen statistics

75d297c

Merge branch 'pick-ray-2' into offscreen-picking

96143eb

Merge branch 'master' into offscreen-picking

5392ed7

Offscreen picking

c389629

Added Sandcastle demo

8b88951

lilleyse added 2 commits October 5, 2018 10:05

Cleaned up Ray.clone

8273d8e

Update frame number and time in main render loop so changes are refle…

eaa858b

…cted in updates.

pjcozzi reviewed Oct 7, 2018

View reviewed changes

This was referenced Oct 12, 2018

sampleHeight bug #7120

Closed

Possible fix for possible scene.pickFromRay bug #7129

Closed

lilleyse force-pushed the offscreen-picking branch from ebb1f3c to 981273b Compare October 15, 2018 19:19

Fix #7120 by picking through primitives that don't write depth for sa…

93061df

…mpleHeight and clampToHeight

lilleyse force-pushed the offscreen-picking branch from 981273b to 93061df Compare October 15, 2018 19:22

lilleyse added 3 commits October 15, 2018 15:42

Merge branch 'master' into offscreen-picking

336bf3a

Merge branch 'master' into offscreen-picking

b950f17

Added #7120 to CHANGES

af109b2

Modify arrays in place

320bf99

lilleyse force-pushed the offscreen-picking branch from b7852e9 to 320bf99 Compare October 16, 2018 17:26

lilleyse added 4 commits October 16, 2018 13:43

Rename demo

751b138

Merge branch 'master' into offscreen-picking

003baa5

Fix tests

b605450

Added pick tests

bdd2d78

lilleyse added 8 commits November 5, 2018 18:07

Add terrain to Sandcastle demo

470bd0f

Remove minimumGeometricError

d0c16e3

Formatting

e457213

Renamed asyncLoaders to asyncRayPicks

9a2e7eb

Scene updates

3d64294

Merge branch 'master' into offscreen-picking

ac9a986

Add to doc

b5cafd7

Merge conflict fix

3876766

lilleyse changed the base branch from offscreen-picking-prep to master November 6, 2018 00:59

Merge branch 'master' into offscreen-picking

f461329

Updated CHANGES.md

d115e17

lilleyse mentioned this pull request Nov 7, 2018

Move picking code into its own file #7247

Closed

Merge branch 'master' into offscreen-picking

9956a08

pjcozzi approved these changes Nov 8, 2018

View reviewed changes

pjcozzi merged commit 97159a4 into master Nov 8, 2018

pjcozzi deleted the offscreen-picking branch November 8, 2018 00:27

lilleyse mentioned this pull request Nov 8, 2018

Create uniform buildings CesiumGS/3d-tiles-validator#159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clamp to 3D Tiles most detailed #7115

Clamp to 3D Tiles most detailed #7115

lilleyse commented Oct 3, 2018 •

edited

Loading

cesium-concierge commented Oct 3, 2018

pjcozzi commented Oct 7, 2018

pjcozzi Oct 7, 2018

pjcozzi Oct 7, 2018

OmarShehata Oct 9, 2018

lilleyse Oct 16, 2018

OmarShehata Oct 16, 2018

OmarShehata commented Oct 9, 2018 •

edited

Loading

sherousee commented Oct 15, 2018

lilleyse commented Oct 15, 2018 •

edited

Loading

lilleyse commented Oct 15, 2018 •

edited

Loading

lilleyse commented Oct 15, 2018

lilleyse commented Nov 6, 2018

pjcozzi commented Nov 6, 2018

pjcozzi commented Nov 6, 2018

pjcozzi commented Nov 6, 2018

lilleyse commented Nov 7, 2018

lilleyse commented Nov 7, 2018

lilleyse commented Nov 7, 2018

lilleyse commented Nov 7, 2018

pjcozzi commented Nov 7, 2018

pjcozzi commented Nov 7, 2018

pjcozzi commented Nov 7, 2018

lilleyse commented Nov 7, 2018

pjcozzi commented Nov 8, 2018

Clamp to 3D Tiles most detailed #7115

Clamp to 3D Tiles most detailed #7115

Conversation

lilleyse commented Oct 3, 2018 • edited Loading

cesium-concierge commented Oct 3, 2018

pjcozzi commented Oct 7, 2018

pjcozzi Oct 7, 2018

Choose a reason for hiding this comment

pjcozzi Oct 7, 2018

Choose a reason for hiding this comment

OmarShehata Oct 9, 2018

Choose a reason for hiding this comment

lilleyse Oct 16, 2018

Choose a reason for hiding this comment

OmarShehata Oct 16, 2018

Choose a reason for hiding this comment

OmarShehata commented Oct 9, 2018 • edited Loading

sherousee commented Oct 15, 2018

lilleyse commented Oct 15, 2018 • edited Loading

lilleyse commented Oct 15, 2018 • edited Loading

lilleyse commented Oct 15, 2018

lilleyse commented Nov 6, 2018

pjcozzi commented Nov 6, 2018

pjcozzi commented Nov 6, 2018

pjcozzi commented Nov 6, 2018

lilleyse commented Nov 7, 2018

lilleyse commented Nov 7, 2018

lilleyse commented Nov 7, 2018

lilleyse commented Nov 7, 2018

pjcozzi commented Nov 7, 2018

pjcozzi commented Nov 7, 2018

pjcozzi commented Nov 7, 2018

lilleyse commented Nov 7, 2018

pjcozzi commented Nov 8, 2018

lilleyse commented Oct 3, 2018 •

edited

Loading

OmarShehata commented Oct 9, 2018 •

edited

Loading

lilleyse commented Oct 15, 2018 •

edited

Loading

lilleyse commented Oct 15, 2018 •

edited

Loading